Add basic OpenTelemetry tracing for client and server requests#2381
Add basic OpenTelemetry tracing for client and server requests#2381
Conversation
Add opentelemetry-api as an optional dependency (mcp[otel]) and create
spans for client request/response cycles and server request handling.
Span names follow the pattern:
- Client: "MCP {method} {target}" (e.g. "MCP tools/call my_tool")
- Server: "MCP handle {method} {target}"
When opentelemetry-api is not installed, tracing is a complete no-op
with zero overhead.
Simplify _otel.py by removing the ImportError fallback and lru_cache since opentelemetry-api is now always available.
Both client and server can send requests, so use 'MCP send' for the sending side and 'MCP handle' for the receiving side consistently.
Inject traceparent/tracestate into request _meta on send, extract it on the server side as the parent context for the handler span. This enables cross-process trace propagation over stdio and HTTP transports.
The lowest-direct CI resolution may not install a logfire version that includes the testing module.
…python-sdk into marcelo/otel-spans
| # Inject W3C trace context into _meta (SEP-414). | ||
| meta: dict[str, Any] = request_data.setdefault("params", {}).setdefault("_meta", {}) | ||
| inject_trace_context(meta) |
There was a problem hiding this comment.
🟡 Nit: request_data.setdefault("params", {}).setdefault("_meta", {}) unconditionally adds params: {"_meta": {}} to every request, even paramless ones like ping and even when no OTel SDK is configured (where inject() is a no-op). Consider guarding the setdefault chain so _meta is only created when there is an active trace context to inject.
Extended reasoning...
What the bug is
At line 280-282 of session.py, inside send_request, the code unconditionally calls:
meta: dict[str, Any] = request_data.setdefault("params", {}).setdefault("_meta", {})
inject_trace_context(meta)This creates both a params key and a nested _meta key on every outgoing request, regardless of whether the request originally had params or whether an OTel SDK is actually configured.
How it manifests
Consider a PingRequest, which has params=None. When serialized via model_dump(exclude_none=True), the resulting request_data dict has no params key at all. Before this PR, the wire format would be:
{"jsonrpc": "2.0", "id": 0, "method": "ping"}After this PR, the setdefault chain inserts params: {"_meta": {}}, changing the wire format to:
{"jsonrpc": "2.0", "id": 0, "method": "ping", "params": {"_meta": {}}}When no OTel SDK is installed (the default case, since opentelemetry-api provides only a no-op tracer), inject() writes nothing into the dict, leaving an empty _meta: {} in every request.
Why existing code doesn't prevent it
The setdefault calls are placed unconditionally inside the otel_span context manager, with no check for whether there is actually a valid span context to propagate. The inject_trace_context wrapper simply calls inject(meta) without any conditional logic.
Impact
The practical impact is low. _meta is a well-defined field in the MCP protocol, and RequestParams validates {"_meta": {}} correctly since all RequestParamsMeta fields are NotRequired. No server should break from receiving this. However, it adds unnecessary bytes to every single request on the wire, even when tracing is completely unused, and it represents an unnecessary behavioral change to the wire format.
Suggested fix
Only create the _meta dict when there is an active span context worth propagating. For example, inject into a temporary dict first and only merge it into request_data if the dict is non-empty after injection:
tmp: dict[str, Any] = {}
inject_trace_context(tmp)
if tmp:
request_data.setdefault("params", {}).setdefault("_meta", {}).update(tmp)Step-by-step proof
- Create a
PingRequestwithparams=None. model_dump(exclude_none=True)produces{"method": "ping"}.request_data.setdefault("params", {})inserts"params": {}and returns{}..setdefault("_meta", {})inserts"_meta": {}into that new params dict.inject_trace_context(meta)callsinject({})which, with no OTel SDK, is a no-op.- The final
request_datais{"method": "ping", "params": {"_meta": {}}}with extra fields that were never there before.
There was a problem hiding this comment.
An empty _meta: {} is harmless per the MCP spec. When no OTel SDK is configured, inject() is already a no-op (writes nothing). Guarding this adds complexity for no benefit.
| task_support = self._experimental_handlers.task_support if self._experimental_handlers else None | ||
| # Get task metadata from request params if present | ||
| task_metadata = None | ||
| if hasattr(req, "params") and req.params is not None: # pragma: no branch |
There was a problem hiding this comment.
🟡 # pragma: no branch on line 482 is incorrect — PingRequest has params: RequestParams | None = None, so when a ping arrives req.params is None and the false branch is genuinely taken. Per CLAUDE.md, this pragma should only be used for coverage.py ->exit arc misreports on nested async with, not for real conditional branches. Remove the pragma to avoid masking missing test coverage for the None-params path.
Extended reasoning...
What the bug is
The PR adds # pragma: no branch to the condition if hasattr(req, "params") and req.params is not None: at server.py:482. This pragma tells coverage.py that the false branch of this conditional is never taken, suppressing any coverage warning for the missing branch.
Why this is incorrect
PingRequest is defined as Request[RequestParams | None, ...] with params: RequestParams | None = None (see types/_types.py:589). When the server handles a ping request, req.params IS None, so the condition evaluates to False and the false branch is genuinely executed — task_metadata stays None and execution falls through to constructing the ServerRequestContext.
Step-by-step proof
- A client sends a
PingRequestwith defaultparams=None. - The server dispatches to
_handle_requestwith this request. - At line 482,
hasattr(req, "params")isTrue(PingRequest always has aparamsattribute), butreq.params is not NoneisFalse. - The overall condition is
False, so the body (task_metadata = getattr(req.params, "task", None)) is skipped. task_metadataremainsNonefrom its initialization on line 481.- However,
# pragma: no branchtells coverage.py this false branch never happens, which is wrong.
Project convention violated
The project's CLAUDE.md (lines 55-56) explicitly states that # pragma: no branch should only be used when coverage.py misreports the ->exit arc for nested async with blocks on Python 3.11+. This is a genuine conditional branch, not an async-with exit arc issue.
Impact
The impact is limited to test coverage reporting — this has zero runtime effect. The pragma was not present in the original code (visible in the diff) and was newly added by this PR. It could mask the fact that tests don't exercise the None-params path through this conditional, defeating the purpose of the project's 100% branch coverage requirement.
Fix
Simply remove the # pragma: no branch comment from line 482. If coverage then reports the false branch as uncovered, add a test that exercises a PingRequest (which has params=None) to cover it properly.
There was a problem hiding this comment.
Pre-existing pragma, not introduced by this PR.
Summary
Adds OpenTelemetry tracing to MCP client and server request handling, with W3C trace context propagation via
_metaper SEP-414.opentelemetry-apiis added as a mandatory dependency. It provides a no-op tracer by default - spans are only recorded when the user configures an OTel SDK (e.g. via Logfire, Jaeger, OTLP exporters).Spans
send_request):MCP send tools/call my_toolwithSpanKind.CLIENT_handle_request):MCP handle tools/call my_toolwithSpanKind.SERVERServer spans record
ERRORstatus when the handler returnsErrorData. Client spans record errors automatically via OTel'sstart_as_current_spanwhenMCPErrorpropagates.Trace context propagation (SEP-414)
On send,
traceparent/tracestateare injected into_metaof the JSON-RPC request. On the server side,_metais extracted and used as the parent context for the handler span. This enables cross-process trace propagation over stdio and HTTP transports.Attributes
mcp.method.name- the JSON-RPC method (e.g.tools/call)jsonrpc.request.id- the request IDNew files
src/mcp/shared/_otel.py- OTel helpers (otel_span,inject_trace_context,extract_trace_context)tests/shared/test_otel.py- E2E test verifying span names, kinds, attributes, and trace propagationTest plan